feat(permissions): Add sorting feature for Users based on role#7654
feat(permissions): Add sorting feature for Users based on role#7654afsuyadi wants to merge 2 commits into
Conversation
|
@afsuyadi is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
the
front-end
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces role-based sorting for users in both the EditPermissions and OrganisationUsersTable components. The review feedback highlights a performance bottleneck in EditPermissions where an O(N * M) lookup is performed on every render; this can be optimized to O(N + M) by using a Set of admin user IDs. Additionally, the reviewer recommends defining the static sorting options arrays outside of both components to provide stable references and prevent breaking memoization in child components.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
talissoncosta
left a comment
talissoncosta
left a comment
There was a problem hiding this comment.
Thanks for this @afsuyadi 🙏 Nice addition.
Could you go over the inline suggestions from Gemini Code Assist and address them? The main ones are hoisting the sort-option arrays to module scope (stable references so they don't break PanelSearch's memoisation) and the admin-lookup optimisation in EditPermissions.
Also, mind running npm run lint:fix and pushing the result? Wrapping the render prop in { … return ( … ) } left the JSX under-indented, so a format pass will tidy the diff.
Thanks!
Okay thanks, sure do 👍 |
|
Okay I have addressed the suggestions from Gemini Code Assistant and conducted the linting. Please inform me if there are more to be done. Thanks for the review @talissoncosta . |
2 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #7613
Users in organisation, project, and environment permissions settings can now be sorted by role, making it easy to distinguish admisnitrators from standard users at a glance.
Each user is assigned a role rank for sorting:
Sorting ASC puts administrators at the top; toggling reverses the order.
How did you test this code?
Tested manually on a local dev environment:
Screencast+from+01-06-26+14_56_46.mp4